Fixed rspec --bisect to sort ids_to_run as the original order #3093
Conversation
Do you think it’s possible to cover the 4.rb 2.rb case that you mention in the description in a spec that would be red before the fix? We usually do it ourselves, but I couldn’t find your real name. May I kindly ask you to add a Changelog entry? |
Ahh, I apologize the lack of explanation 🙏 And I tried to add a changelog! If I should squash commits, I'll do :) |
|
If we add the |
|
I see. For example, next spec fails when we add it do
all_ids = %w[ 1.rb[1] 2.rb[1] f.rb[1] 3.rb[1] ]
dependent_ids = %w[ 2.rb[1] ]
failure_id = "f.rb[1]"
patterns = [
[all_ids, [failure_id]],
[[failure_id], []],
[all_ids & (all_ids.slice(all_ids.size/2 .. -1) + [failure_id]), []],
[all_ids & (all_ids.slice(0 .. all_ids.size/2) + [failure_id]), [failure_id]],
[all_ids & (all_ids.slice(all_ids.size/2/2 .. all_ids.size/2) + [failure_id]), [failure_id]]
]
allow(RSpec::Core::Bisect::ExampleSetDescriptor).to receive(:new).and_call_original
patterns.each { |pattern|
expect(RSpec::Core::Bisect::ExampleSetDescriptor).to receive(:new).with(*pattern)
}
fake_runner = FakeBisectRunner.new(
all_ids,
[],
{ failure_id => dependent_ids }
)
minimizer = new_minimizer(fake_runner)
minimizer.find_minimal_repro
expect(minimizer.repro_command_for_currently_needed_ids).to eq("rspec 2.rb[1] f.rb[1]")
endThis spec tests a test helper FakeBisectRunner's internal logic, so I'm not sure this spec should be added. |
|
Alright. But we've changed how
Is it possible? Does it make sense? Does it apply in this case? |
|
Sorry for my bad explanation 🙏 First, because spec helper (FakeBisectRunner) has a tiny bug and I fixed it removing unnecessary So, the bug in the spec helper was getting in the way, but I think the spec already has a way to reliably reproduce the problem. I would be grateful if you could let me know if this doesn't seem to match your intentions 🙏 For documentation purpose the following spec could be added, but it would overlap with this spec. it 'returns the reproducing command even if there are unrelated ids after the failure id' do
fake_runner = FakeBisectRunner.new(
%w[ 1.rb[1] 2.rb[1] 3.rb[1] 4.rb[1] ],
[],
{ "3.rb[1]" => %w[ 2.rb[1] ] }
)
minimizer = new_minimizer(fake_runner)
minimizer.find_minimal_repro
expect(minimizer.repro_command_for_currently_needed_ids).to eq("rspec 2.rb[1] 3.rb[1]")
end |
|
We have integration specs for the bisect runners it would be good to add a check to that which would fail if this wasn't implemented |
|
Thank you! I added an integration spec. 6877eb8 |
makicamel
force-pushed
the
fix-non_failing_example_ids
branch
2 times, most recently
from
6877eb8 to
f5d8e54
Compare
|
Can you rebase this once #3097 is merged / or cherry-pick that commit, theres an issue with our legacy actions needing fixing. |
…ent`, minimizer checks the execution order of the devided ids are the same as the original order. So we need to sort them as the original order before run. For example, When we have 4 examples: 1.rb, 2.rb, 3.rb, 4.rb and 3.rb is failure order-dependent with 2.rb. We expect to minimizer reproduce command `rspec 2.rb[1] 3.rb[1]`, but minimizer raises 'The example ordering is inconsistent.' This is because the ids to run are in the order 4.rb 2.rb (diffrent from the original), so fixed to sort them in the same order as the original error.
makicamel
force-pushed
the
fix-non_failing_example_ids
branch
from
f5d8e54 to
96e5ad1
Compare
|
@JonRowe |
|
Thanks |
Fixed rspec --bisect to sort ids_to_run as the original order
|
Released in 3.13.1 apologies for the delay |
In
RSpec::Core::Bisect::ExampleMinimizer#abort_if_ordering_inconsistent, minimizer checks the execution order of the devided ids are the same as the original order. So we need to sort them as the original order before run.For example,
When we have 4 examples: 1.rb, 2.rb, 3.rb, 4.rb and 3.rb is failure order-dependent with 2.rb.
We expect to minimizer reproduce command
rspec 2.rb[1] 3.rb[1], but minimizer raises 'The example ordering is inconsistent.'This is because the ids to run are in the order 4.rb 2.rb (diffrent from the original), so fixed to sort them in the same order as the original error.